Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: perfection tracker for levels with best number of commands #1204

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

Patolord
Copy link
Contributor

This feature introduces the ability to track and record the user's best performance on each level. By storing both the completion status (solved) and whether the user achieved the best possible outcome (best), the application can provide more detailed feedback and track user improvement over time.

Copy link

netlify bot commented Nov 18, 2024

Deploy Preview for xenodochial-hugle-b9ec84 ready!

Name Link
🔨 Latest commit 928577b
🔍 Latest deploy log https://app.netlify.com/sites/xenodochial-hugle-b9ec84/deploys/673b38308a40d50008b08cf5
😎 Deploy Preview https://deploy-preview-1204--xenodochial-hugle-b9ec84.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pcottle
Copy link
Owner

pcottle commented Nov 18, 2024

Oh cool feature! Do you have a screenshot of what the UI looks like?

My only worry is this part:

function _syncToStorage() {
try {
localStorage.setItem(SOLVED_MAP_STORAGE_KEY, JSON.stringify(_solvedMap));
} catch (e) {
console.warn('local storage failed on set', e);
}
}

Since we sync that map to local storage, I think this existing code would wipe out the existing solved map status.

I think we can fix that in the isLevelSolved: function(levelID) { function @Patolord

@pcottle pcottle merged commit a83e995 into pcottle:main Nov 18, 2024
5 checks passed
pcottle pushed a commit that referenced this pull request Nov 18, 2024
@Patolord
Copy link
Contributor Author

I did not mess too much with styling to not ruin your design so the css just change to gold the level.
but it would be cool to change icon to star. ( i did not fix the hover and other css classes).
gitbranching
Do you mean we need to migrate old users ?
from
{"intro1": true, "intro2": true}
to
{
"intro1": { "solved": true, "best": true },
"intro2": { "solved": true, "best": false }
} ?

@pcottle
Copy link
Owner

pcottle commented Nov 18, 2024

A star would be cool!!

I added a fix for old users:
b3d0004

it works now :)
https://learngitbranching.js.org/?NODEMO&command=levels

Screenshot 2024-11-18 at 6 47 35 PM

@Patolord
Copy link
Contributor Author

its just a matter of changing react-ok-icon to react-star class on the tag.
but im not sure how to implement the conditional styling.

@Patolord Patolord deleted the perfectionTracker branch November 19, 2024 00:00
@pcottle
Copy link
Owner

pcottle commented Nov 19, 2024

That would be in the template.index.html, over in this area:

    <div class="iconHolder box horizontal">
      <% for (var i = 0; i < ids.length; i++) { %>
        <a href="javascript:void(0)" class="levelIcon box center centerAlign vertical" id="levelIcon-<%=ids[i]%>" data-id="<%=ids[i]%>">
          <div class="index box" data-id="<%=ids[i]%>">
            <i class="icon-ok-circle" data-id="<%=ids[i]%>"></i>
            <div class="indexNum" data-id="<%=ids[i]%>">
              <%= i + 1 %>
            </div>
          </div>
        </a>
      <% } %>
    </div>

I would add another icon and then just conditionally show or hide the icon based on CSS rules :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants